chore(maintenance): avoid attaching two middlewares to ua #1583
Conversation
boring-cyborg
bot
added
commons
| @@ -33,6 +33,13 @@ const customUserAgentMiddleware = (feature: string) => { | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
not sure I like ignoring things in the type system
There was a problem hiding this comment.
Acknowledged and agree.
In this instance we decided to use an any as a type for the client because we didn't want to bring in the @aws-sdk/types package into the utility. This code resides in the commons utility, which is pulled in by all the other Powertools utilities.
Given that there's all the core utilities don't use any AWS SDK, using the @aws-sdk/types in this module would require customers to install it as a developer dependency when they develop or build their code using Powertools.
This is a problem because when building with TypeScript, the type-checking engine will block the compilation is the dependency is missing.
To avoid this, we decided to go for an any type (which is discouraged in our codebase - hence the @ts-ignore) and also put unit tests.
We have considered writing our own minimal Client type, and the option is still in the cards, but have decided to not include it for now given the limited scope of the feature (i.e. getting a client from a 3rd party library and calling a method on it) and the fact that we implemented the function in a way that even if you pass a random object (aka not a client) it won't throw an error but just log a warning.
I am still strongly inclined to review this type as soon as we have bandwidth and remove the ignore.
There was a problem hiding this comment.
I think my preference would be to write our own types def.
Description of your changes
This PR adds a check before adding user agent middleware, to avoid attaching two or more middlewares and fire a warning in the catch block. I have also refactored the tests, to make it a bit more readable.
Related issues, RFCs
Issue number: closes #1578
Checklist
Breaking change checklist
Is it a breaking change?: NO
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.